OSAC-1533: Fix --network-attachment ignored with --catalog-item by obochan-rh · Pull Request #705 · osac-project/fulfillment-service

obochan-rh · 2026-06-15T18:32:59Z

Summary

buildSpecFromCatalogItem() was missing the applyNetworkingFlags() call, causing --network-attachment flags to be silently dropped when creating compute instances via --catalog-item
The sibling buildSpec() function (used by --template) already included this call
Added unit tests for buildSpecFromCatalogItem to verify network attachments are correctly applied

Root Cause

When --catalog-item support was added, buildSpecFromCatalogItem() duplicated all optional flag handling from buildSpec() (image, cores, memory, ssh-key, disks, etc.) but omitted the applyNetworkingFlags() call at the end. The CLI accepted --network-attachment without error but never included it in the gRPC request.

Fix

Added applyNetworkingFlags(&spec) in buildSpecFromCatalogItem() before returning — same pattern as buildSpec() (3 lines).

Test Plan

Existing unit tests pass (20/20 specs, was 18 before)
New buildSpecFromCatalogItem tests verify network attachments are populated
Tested on edge-04: error changed from "at least one network attachment is required" (network attachment dropped) to "subnet does not exist" (network attachment correctly sent to server)

Fixes: https://redhat.atlassian.net/browse/OSAC-1533

Summary by CodeRabbit

New Features
- Added support for network-attachment flags when creating compute instances from catalog items, ensuring the instance specification reflects the provided network attachment values.
Tests
- Expanded catalog-item creation tests to validate correct network-attachment parsing (including subnet=...,security-groups=... entries), proper omission when no flags are set, and error handling for invalid key/value formats.

The buildSpecFromCatalogItem() function was missing the applyNetworkingFlags() call, causing --network-attachment flags to be silently dropped when creating compute instances via --catalog-item. The sibling buildSpec() function (used by --template) already includes this call. Signed-off-by: Ofer Bochan <obochan@redhat.com>

openshift-ci-robot · 2026-06-15T18:33:03Z

@obochan-rh: This pull request references OSAC-1533 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

buildSpecFromCatalogItem() was missing the applyNetworkingFlags() call, causing --network-attachment flags to be silently dropped when creating compute instances via --catalog-item

The sibling buildSpec() function (used by --template) already included this call

Added unit tests for buildSpecFromCatalogItem to verify network attachments are correctly applied

Root Cause

When --catalog-item support was added, buildSpecFromCatalogItem() duplicated all optional flag handling from buildSpec() (image, cores, memory, ssh-key, disks, etc.) but omitted the applyNetworkingFlags() call at the end. The CLI accepted --network-attachment without error but never included it in the gRPC request.

Fix

Added applyNetworkingFlags(&spec) in buildSpecFromCatalogItem() before returning — same pattern as buildSpec() (3 lines).

Test Plan

Existing unit tests pass (20/20 specs, was 18 before)

New buildSpecFromCatalogItem tests verify network attachments are populated

Tested on edge-04: error changed from "at least one network attachment is required" (network attachment dropped) to "subnet does not exist" (network attachment correctly sent to server)

Fixes: https://redhat.atlassian.net/browse/OSAC-1533

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-06-15T18:33:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: obochan-rh
Once this PR has been reviewed and has the lgtm label, please assign ygalblum for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

coderabbitai · 2026-06-15T18:33:13Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: d25e9349-02dd-4861-bd39-82741ac1fa8d

📥 Commits

Reviewing files that changed from the base of the PR and between 6f07494 and e55ffc9.

📒 Files selected for processing (1)

internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go

Walkthrough

buildSpecFromCatalogItem now calls applyNetworkingFlags on the constructed ComputeInstanceSpec and propagates any error before building the final spec. Tests replace the prior buildSpec attachment test with a buildSpecFromCatalogItem suite covering both populated and empty network attachment flag cases, plus error validation.

Changes

Network Attachment Flags in Catalog-Item Spec Builder

Layer / File(s)	Summary
`applyNetworkingFlags` wired into `buildSpecFromCatalogItem` with tests `internal/cmd/cli/create/computeinstance/create_compute_instance_cmd.go`, `internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go`	`buildSpecFromCatalogItem` now invokes `applyNetworkingFlags(&spec)` and returns any error before calling `spec.Build()`. Tests verify that `NetworkAttachments` are parsed and populated from `runnerContext.args.networkAttachments` (bare subnet ID and `subnet=...,security-groups=...` form), that the field is absent when no flags are provided, and that invalid key/value fragments are caught. Risk severity: patched — previously catalog-item creation silently bypassed network attachment flags, risking misconfigured NICs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

⚠️ A network flag left unapplied,
Could leave your NICs misconfigured, untried.
Now applyNetworkingFlags gets its due call,
Catalog items stand guarded — attached, one and all.
Risk severity: patched. The blast radius: small. 🛡️

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main bug fix: that --network-attachment flags were being ignored when using --catalog-item option.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets	✅ Passed	No hardcoded secrets found. PR adds networking flag handling and unit tests using only placeholder test data and function calls; no API keys, tokens, passwords, credentials, or suspicious string li...
No-Weak-Crypto	✅ Passed	No cryptographic operations, weak crypto algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons detected in the PR changes.
No-Injection-Vectors	✅ Passed	No injection vectors detected. Code contains no SQL concatenation, shell execution, eval/exec, unsafe deserialization, or dynamic code execution with user input. Data is properly validated and seri...
Container-Privileges	✅ Passed	PR contains only Go source code and unit tests; no container/K8s manifests present to check for privilege escalation risks.
No-Sensitive-Data-In-Logs	✅ Passed	PR adds only 3 lines calling existing applyNetworkingFlags() and unit tests. No new logging statements introduced. SSH keys and network IDs are processed but never logged.
Ai-Attribution	✅ Passed	No AI tool usage is mentioned in PR description or commits, and no Assisted-by/Generated-by/Co-Authored-By trailers are present. The "AI-generated summary" in raw_summary is meta-commentary from ex...

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go`:
- Around line 76-103: Add a new test case (It block) to the
buildSpecFromCatalogItem describe suite that validates error handling for
invalid input. Create a runnerContext with invalid or malformed values in the
networkAttachments field (such as a string that cannot be parsed correctly),
then call buildSpecFromCatalogItem and assert that it returns an error using
Expect(err).To(HaveOccurred()). This ensures the function properly rejects and
reports invalid network attachment input rather than silently ignoring it.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0c1643c5-4e52-4a45-8607-79571a46f6bf

📥 Commits

Reviewing files that changed from the base of the PR and between dc987c9 and 6f07494.

📒 Files selected for processing (2)

internal/cmd/cli/create/computeinstance/create_compute_instance_cmd.go
internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go

Verify that --network-attachment flags are correctly applied when using --catalog-item, matching the existing buildSpec test coverage. Signed-off-by: Ofer Bochan <obochan@redhat.com>

openshift-ci-robot added the jira/valid-reference label Jun 15, 2026

openshift-ci Bot requested review from akshaynadkarni and jhernand June 15, 2026 18:33

coderabbitai Bot requested changes Jun 15, 2026

View reviewed changes

Comment thread internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go

OSAC-1533: Add unit tests for buildSpecFromCatalogItem networking

e55ffc9

Verify that --network-attachment flags are correctly applied when using --catalog-item, matching the existing buildSpec test coverage. Signed-off-by: Ofer Bochan <obochan@redhat.com>

obochan-rh force-pushed the OSAC-1533/fix-catalog-item-network-attachment branch from 6f07494 to e55ffc9 Compare June 16, 2026 05:46

coderabbitai Bot approved these changes Jun 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSAC-1533: Fix --network-attachment ignored with --catalog-item#705

OSAC-1533: Fix --network-attachment ignored with --catalog-item#705
obochan-rh wants to merge 2 commits into
osac-project:mainfrom
obochan-rh:OSAC-1533/fix-catalog-item-network-attachment

obochan-rh commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

openshift-ci-robot commented Jun 15, 2026 •

edited by openshift-ci Bot

Loading

Summary

Root Cause

Fix

Test Plan

Uh oh!

openshift-ci Bot commented Jun 15, 2026

Uh oh!

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

obochan-rh commented Jun 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Test Plan

Summary by CodeRabbit

Uh oh!

openshift-ci-robot commented Jun 15, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Test Plan

Uh oh!

openshift-ci Bot commented Jun 15, 2026

Uh oh!

coderabbitai Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

obochan-rh commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

openshift-ci-robot commented Jun 15, 2026 •

edited by openshift-ci Bot

Loading

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading